feat: Make navigation looping configurable #9511
Conversation
| ): IFocusableNode | null { | ||
| if (!node || (!loop && this.getLastNode() === node)) return null; | ||
| const originalLoop = this.getNavigationLoops(); | ||
| this.setNavigationLoops(loop); |
There was a problem hiding this comment.
this whole rigamarole is unfortunate, is the loop parameter in getNextNode already released? should we just deprecate it? or potentially should get navigationLoops policy be made part of the navigation policy instead and it can pass that value into here? (This suggestion may not make sense, I forget which order these calls happen in, let me know if you want me to dig in further to find a proposal that might make more sense)
There was a problem hiding this comment.
Yeah I don't super love it, but yes that argument is public already, hence this stack-y thing. I think the cursor feels like the right level to control this (the navigation policies should just be given X, what's the next/previous thing? with filtering and larger-scale behavior delegated to the cursor), but the existence of this argument does complicate it.
There was a problem hiding this comment.
Can you add a comment with a TODO so we can come back to this when we're designing the eventual "real" solution? The argument should probably just be removed at that point
There was a problem hiding this comment.
Done, and yeah if we're comfortable doing that that SGTM.
gonfunko
merged commit 8522bff
into
add-screen-reader-support-experimental
The basics
The details
Resolves
Fixes part of #9496
Proposed Changes
This PR makes the looping behavior in
LineCursorconfigurable. AlthoughgetPreviousNode()andgetNextNode()already allowed configuring this,in(),out(),prev()andnext()did not. This PR keeps the existing behavior, but will allow the keyboard experiment to disable looping and play an alert when the end of the navigable tree is reached.